Add support for Sve.UnzipEven/Odd & Sve.ZipHighLow #101294
Conversation
|
Note regarding the |
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics |
|
On a V1 system with SVE vector length 256 bits, tests for this patch would fail. We are currently limited to vector length of 128 causing incorrect results on systems with higher vector lengths. We need #101295 to limit vector length to 128 bits. |
|
@a74nh @kunalspathak @dotnet/arm64-contrib @arch-arm64-sve |
|
Contributes to #99957 |
|
Thanks @SwapnilGaikwad for the PR for bitmanipulate. When you merge the PR, could you check off the boxes from #99957? |
Is your plan to always limit the vector length to 128 bits? If yes, how can we test the functionality on 256 bits and higher? |
src/tests/JIT/HardwareIntrinsics/Arm/Shared/_BinaryOpSveTestTemplate.template
Outdated
Show resolved
Hide resolved
I'll post a comment on the issue as I cannot edit the text of the issue 👍 |
This just for temporary workaround until we support higher vector lengths using Vector. Currently it's limited to 128bits. These changes are tested for higher vector lengths locally. Once we start supporting higher vector lengths with Vector, we don't need to limit the vector lengths using the mentioned patch. There won't be any changes required to this patch then. |
|
The only failure that BuildAnalysis didn't tag as "known issue" is actually #91757 and not sure why it didn't get tagged. cc: @JulieLeeMSFT |
|
/ba-g #91757 |
| /// svbool_t svuzp1_b8(svbool_t op1, svbool_t op2) | ||
| /// UZP1 Presult.B, Pop1.B, Pop2.B | ||
| /// </summary> | ||
| public static unsafe Vector<byte> UnzipEven(Vector<byte> left, Vector<byte> right) => UnzipEven(left, right); |
There was a problem hiding this comment.
I just realized that zip and unzip instructions operate on both vector and predicate registers. How do we know which one to invoke? Currently, we will always invoke vector variants? @a74nh ?
If we always going to support just the vector version, then please remove the comment for predicate registers.
There was a problem hiding this comment.
Right now it'll just be the vector variant. If we're using masks then there will be a lot of conversions and the vector version will be used.
Raised a ticket: #101598
In the meantime, is it ok to leave the comment as is?
There was a problem hiding this comment.
Thanks for opening an issue and yes, ok to keep it as of now.
* Add support for Sve.UnzipEven/Odd & Sve.ZipHighLow * Rename the test template --------- Co-authored-by: Kunal Pathak <[email protected]>
* Add support for Sve.UnzipEven/Odd & Sve.ZipHighLow * Rename the test template --------- Co-authored-by: Kunal Pathak <[email protected]>
Add SVE API for unpredicated versions of UnzipEven, UnzipOdd, ZipHigh & ZipLow methods.